Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

⬆️ Update pytest-asyncio to 0.23.4 #2764

Merged
merged 9 commits into from
Feb 7, 2024
Merged

⬆️ Update pytest-asyncio to 0.23.4 #2764

merged 9 commits into from
Feb 7, 2024

Conversation

ff137
Copy link
Contributor

@ff137 ff137 commented Feb 7, 2024 •
edited
Loading

Summary:
⬆️ Upgrades pytest-asyncio to latest
✅ fix some test's event loop scopes (also mark some tests requiring askar or bbs)
🎉 resolves 2154 deprecation warnings

Original:
Reviewing test logs show that there are 2154 deprecation warnings coming from across 202 test files, each reporting:

  /home/runner/.cache/pypoetry/virtualenvs/aries-cloudagent-LQSjsNdA-py3.9/lib/python3.9/site-packages/pytest_asyncio/plugin.py:451: DeprecationWarning: pytest-asyncio detected an unclosed event loop when tearing down the event_loop
  fixture: <_UnixSelectorEventLoop running=False closed=False debug=False>
  pytest-asyncio will close the event loop for you, but future versions of the
  library will no longer do so. In order to ensure compatibility with future
  versions, please make sure that:
      1. Any custom "event_loop" fixture properly closes the loop after yielding it
      2. The scopes of your custom "event_loop" fixtures do not overlap
      3. Your code does not modify the event loop in async fixtures or tests

This indicates that the pytests are not configured correctly, or at least not in an up-to-date way.

These are the current test results after upgrading to the latest pytest-asyncio version

FAILED aries_cloudagent/revocation/models/tests/test_issuer_rev_reg_record.py::TestIssuerRevRegRecord::test_fix_ledger_entry - TypeError: the JSON object must be str, bytes or bytearray, not MagicMock
FAILED aries_cloudagent/vc/vc_ld/tests/test_manager.py::test_issue - pyld.jsonld.JsonLdError: ('Could not expand input before compaction.',)
FAILED aries_cloudagent/vc/vc_ld/tests/test_manager.py::test_issue_ed25519_2020 - pyld.jsonld.JsonLdError: ('Could not expand input before compaction.',)
FAILED aries_cloudagent/vc/vc_ld/tests/test_manager.py::test_issue_bbs - TypeError: sequence item 1: expected a bytes-like object, MagicMock found
FAILED aries_cloudagent/vc/vc_ld/tests/test_manager.py::test_store - pyld.jsonld.JsonLdError: ('Could not expand input before compaction.',)
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_repr - TypeError: catching classes that do not inherit from BaseException is not allowed
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_handle_parser_error - TypeError: catching classes that do not inherit from BaseException is not allowed
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_sorting_vcrecord - TypeError: catching classes that do not inherit from BaseException is not allowed
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_tag_query_valid_and_operator - TypeError: catching classes that do not inherit from BaseException is not allowed
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_store_retrieve - TypeError: catching classes that do not inherit from BaseException is not allowed
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_delete - TypeError: catching classes that do not inherit from BaseException is not allowed
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_search - TypeError: catching classes that do not inherit from BaseException is not allowed
ERROR aries_cloudagent/storage/vc_holder/tests/test_askar_vc_holder.py::TestAskarVCHolder::test_tag_query - TypeError: catching classes that do not inherit from BaseException is not allowed
5 failed, 4490 passed, 598 skipped, 6 xfailed, 238 warnings, 8 errors in 128.86s (0:02:08)

Edit: these new test failures/errors all come down to either pytest scope needing to be a higher level, or tests that should be marked "to skip" (askar or bbs)

@ff137
Copy link
Contributor Author

ff137 commented Feb 7, 2024 •
edited
Loading

@dbluhm I know there are likely other priorities to focus on, but just want to raise this one as a potentially important fix, to get the test framework up to date before the 1.0.0 release. Technically just a "nice to have" to make test logs and other warnings easier to review

I'm not so sure how to tackle this one yet, so I wanted to ask if you have any insight as to what might be necessary here. If it's not apparent then it's no worries, I'll take a deeper look once some bandwidth has freed up

@dbluhm
Copy link
Contributor

dbluhm commented Feb 7, 2024

I agree that taking care of the deprecation warnings is valuable, if only to make developers lives easier. Thanks for your efforts on that front (with this and past PRs)! Yeah, unfortunately, off the rip, nothing immediately stands out to me as to what might be causing these test failures.

@ff137 ff137 force-pushed the upgrade-pytest-asyncio branch from cfc4b5b to c69c713 Compare February 7, 2024 20:09
@ff137
Copy link
Contributor Author

ff137 commented Feb 7, 2024

@dbluhm all good! I think I got to the bottom of it... Some tests needed to be marked for skip, requiring bbs or askar. One test (test_fix_ledger) requires indy_credx to work, but I could re-mock it to work without skipping it. And some tests required higher event loop scope. Not sure why, but that solves it! So I think this successfully upgrades to latest pytest-asyncio release, and resolves ~2100 warnings <:-). Just 245 to go 😂

Please review and see if it's all in order!

@ff137
Update lock file
56c7032 
Signed-off-by: ff137 <[email protected]>
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Feb 7, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@ff137 ff137 marked this pull request as ready for review February 7, 2024 20:23
dbluhm
dbluhm approved these changes Feb 7, 2024
View reviewed changes
Copy link
Contributor

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@swcurran swcurran merged commit da5e01f into openwallet-foundation:main Feb 7, 2024
8 checks passed
@ff137 ff137 mentioned this pull request Feb 12, 2024
Merged
@ff137 ff137 deleted the upgrade-pytest-asyncio branch July 4, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbluhm dbluhm dbluhm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ff137 @dbluhm @swcurran